Description

scripts/security/Test-WorkflowPermissions.ps1 checked one thing: whether a file contained a top-level permissions: block, matched with (?m)^permissions: against raw text. It had no job awareness at all. It scored this repository 100% (64/64) while jobs across six workflows carried no declaration of their own.

The convention in .github/instructions/workflows.instructions.md requires job-level declarations, and docs/security/security-model.md asserted that all workflows already had them. Neither was enforced, and the second was not true.

This change taught the gate to parse each workflow with ConvertFrom-Yaml, enumerate its jobs from the parsed object graph, and apply a four-state classification.

Why row 3 is the one that matters

A measurement of all 65 workflows found 0 absent, 7 empty, 58 populated top-level blocks. A rule built only on row 1 would have been unreachable ΓÇö it would have detected the empty set and shipped as a no-op. Row 3 gives the gate a reachable failure state, and it found exactly one on this tree: .github/workflows/beval.yml job evaluate.

That job now declares contents: read ΓÇö which it already inherited. The effective permission set is identical before and after. No job's privilege was widened anywhere in this change, and no generated gh-aw lock file was edited.

Why row 2 is a deliberate pass

An empty workflow-level block grants nothing, so a job beneath it that declares no block inherits an empty set and holds no scope. This is precisely the distinction the third-party Poutine scanner cannot make ΓÇö its utils.empty() helper treats an empty map identically to an absent one ΓÇö which is why it raised code scanning alert #348 against .github/workflows/pr-review.lock.yml, a file that implements Poutine's own documented remediation. That alert was dismissed as a false positive. This change adds no .poutine.yml suppression; a precise local gate was preferred over silencing a scanner.

Worth stating plainly: an empty workflow-level block is a default, not a ceiling. A job that declares its own block beneath one still receives what it declares, because job-level permissions replace the workflow-level set rather than being capped by it.

Supporting changes

• SecurityClasses.psm1 gains a MissingJobPermissions value in the ViolationType ValidateSet, which would otherwise throw on the first job-level finding. Additive only.
• File-level and job-level compliance are reported as separate metrics so compliance-score and violation-count cannot contradict each other under a mixed model.
• SARIF output declares a rule per violation type, emits a matching ruleId, floors startLine at 1, and normalizes paths to forward slashes. The previous version hardcoded a single ruleId and startLine = 1; since the upload step is continue-on-error, a malformed document would have failed silently.
• workflow-permissions-scan.yml gains the setup-ps-modules step. The script now depends on PowerShell-Yaml, which is not preinstalled on ubuntu-latest.

Reviewer note on compliance-score

The score is now denominated in workflow-level plus job-level checks ΓÇö 257 checks on this tree instead of 64 files. It remains a 0ΓÇô100 percentage, so the downstream contract shape in workflow-permissions-scan.yml and pr-validation.yml is unchanged, but historical scores are not directly comparable.

Two job metrics are published deliberately: 193/193 jobs pass the job-level check, but only 188/193 declare their own block. The remaining five pass by inheriting an empty workflow-level block. Collapsing these into one number would have claimed every job declares permissions, which is false.

Related Issue(s)

Closes #2541

Split out from #2527, which covers the full Poutine scan of main and remains open independently.

Type of Change

Select all that apply:

Code & Documentation:

• Bug fix (non-breaking change fixing an issue)
• New feature (non-breaking change adding functionality)
• Breaking change (fix or feature causing existing functionality to change)
• Documentation update

Infrastructure & Configuration:

• GitHub Actions workflow
• Linting configuration (markdown, PowerShell, etc.)
• Security configuration
• DevContainer configuration
• Dependency update

AI Artifacts:

• Reviewed contribution with hve-builder and addressed all actionable findings
• Copilot instructions (.github/instructions/*.instructions.md)
• Copilot prompt (.github/prompts/*.prompt.md)
• Copilot agent (.github/agents/*.agent.md)
• Copilot skill (.github/skills/*/SKILL.md)
• Copilot hook (.github/hooks/*/*.json)
• Eval spec added/updated for changed AI artifacts (evals/)

Note for AI Artifact Contributors:

• Agents: Research, indexing/referencing other project (using standard VS Code GitHub Copilot/MCP tools), planning, and general implementation agents likely already exist. Review .github/agents/ before creating new ones.
• Skills: Must include both bash and PowerShell scripts. See Skills.
• Model Versions: Only contributions targeting the latest Anthropic and OpenAI models will be accepted. Older model versions (e.g., GPT-3.5, Claude 3) will be rejected.
• See Agents Not Accepted and Model Version Requirements.

Other:

• Script/automation (.ps1, .sh, .py)
• Other (please describe):

Sample Prompts (for AI Artifact Contributions)

The only AI artifact touched is .github/instructions/workflows.instructions.md, which is passive guidance applied by glob rather than an invocable artifact. It gains the per-job permissions requirement and the four-state classification table so the documented rule matches the enforced one.

User Request:
Not directly invocable. The instruction file applies automatically when editing files under .github/workflows/.

Execution Flow:
An agent editing a workflow file receives the permissions convention as context, including the requirement that every job declare its own permissions: block whenever the workflow-level block grants any scope, and the permissions: {} exception.

Output Artifacts:
None. The file contributes guidance only.

Success Indicators:
A workflow authored under this guidance passes npm run lint:permissions without a MissingJobPermissions finding.

For detailed contribution requirements, see:

• Common Standards: docs/contributing/ai-artifacts-common.md - Shared standards for XML blocks, markdown quality, RFC 2119, validation, and testing
• Agents: docs/contributing/custom-agents.md - Agent configurations with tools and behavior patterns
• Prompts: docs/contributing/prompts.md - Workflow-specific guidance with template variables
• Instructions: docs/contributing/instructions.md - Technology-specific standards with glob patterns
• Skills: docs/contributing/skills.md - Task execution utilities with cross-platform scripts

Testing

Automated validation

The pre-remediation failure is deliberate evidence: it demonstrates the gate has a reachable failure state, which the original top-level-only rule did not.

Test coverage added

A case per matrix row; null, flow-style, and scalar permissions value shapes; four-space job indentation; a lock-file-shaped regression asserting row 2 passes; a nested step-level permissions key that must not count as a job declaration; SARIF ruleId resolution and startLine floor; malformed-workflow degradation; a no-backslash assertion on violation paths; a job-line-attribution regression using a workflow_call output that shares a job name; and a metrics case pinning that a job passing by inheritance is not counted as declaring a block.

The existing workflow-with-permissions.yml fixture was itself in the row-3 state. Rather than relax its passing assertion, the fixture was brought into compliance so the assertion stays meaningful. No existing fixture or assertion was deleted.

Security analysis

• No secrets, tokens, credentials, or personal data in the diff.
• No privilege escalation. The single workflow edit is provably privilege-neutral by diff inspection.
• No new dependency. PowerShell-Yaml 0.4.7 is already pinned in scripts/security/ps-module-versions.json and installed via the cached setup-ps-modules composite action.
• No generated lock file edited; no scanner suppression added.

Manual testing

Manual testing was not performed. All verification was automated, plus diff-level inspection of the privilege-neutrality claim.

Checklist

Required Checks

• Documentation is updated (if applicable)
• Files follow existing naming conventions
• Changes are backwards compatible (if applicable)
• Tests added for new functionality (if applicable)

AI Artifact Contributions

• Used hve-builder review mode to review contribution
• Addressed all actionable findings from the hve-builder review
• Verified contribution follows common standards and type-specific requirements

Required Local Checks

The following local-safe validation commands must pass before merging:

• Local validation aggregate: npm run validate:local
• Documentation validation (if docs changed): npm run validate:docs (N/A ΓÇö fails locally at docs:lint because docs/docusaurus dependencies are not installed in this environment; unrelated to this change, which touches no Docusaurus files. See Pin eslint to 9.39.4 in docs/docusaurus and regenerate lockfile (PR #2519) #2534.)
• Spell checking: npm run spell-check
• Link validation: npm run lint:md-links

Security Considerations

• This PR does not contain any sensitive or NDA information
• Any new dependencies have been reviewed for security issues
• Security-related scripts follow the principle of least privilege

Additional Notes

An independent review pass caught a CI-breaking defect

A subagent review of the diff found that workflow-permissions-scan.yml had no PowerShell-Yaml install step. The original script was pure regex with no module dependency; the new one hard-requires the module via both #Requires and Import-Module -ErrorAction Stop. That workflow runs on ubuntu-latest with only a checkout step, so the gate would have failed on every PR ΓÇö and because both upload steps are continue-on-error: true, it could have failed quietly. The same review found the job metric was crediting jobs that declare no block. Both are fixed here, with the metric split into two honest numbers.

Known behavior: unparseable workflows fail closed

A workflow that fails to parse now emits a MissingPermissions violation and error annotation, and -FailOnViolation exits non-zero. This matches the current implementation and tests (including the unparseable-workflow integration test asserting exit code 1).

lint:yaml (actionlint) remains a complementary guardrail in the PR gate, so malformed workflows are covered both by YAML validation and by the permissions gate itself.

Backwards compatibility

The MissingJobPermissions ValidateSet addition is purely additive. compliance-score and violation-count keep their shape and type; only the score denominator changes, which is called out above for anyone tracking the number over time.